test(e2e): guard destructive spend-log truncate behind an explicit opt-in - #33751

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

@greptileai

osv-scan is red from base drift, not this PR. It flags mcp==1.26.0 in uv.lock (GHSA-hvrp-rf83-w775, GHSA-jpw9-pfvf-9f58, GHSA-vj7q-gjh5-988w; fixed in 1.27.2/1.28.1). This branch is a tests/e2e-only change and touches no dependency or lock files (git diff origin/litellm_internal_staging..HEAD lists no uv.lock/pyproject.toml), and the same osv-scan failure is present on every currently open PR. The mcp bump belongs in its own dependency PR on the base branch

Greptile Summary

This PR guards the destructive LiteLLM_SpendLogs truncation behind an explicit E2E_RESET_SPEND_LOGS=1 opt-in, so a routine local or CI run that merely exercises an e2e test against a shared DATABASE_URL can no longer wipe real spend data.

• tests/e2e/e2e_db.py is introduced as the single source of truth for destructive DB helpers; run_spend_log_cleanup requires both the env-var opt-in and a confirmed e2e test run before delegating to reset_spend_logs.
• conftest.pytest_sessionfinish is reduced to a one-call adapter that supplies os.environ.get(...), the session stash flag, and the truncate callable — the sys.path mutation and dynamic import are gone.
• reset_spend_logs is moved from spend_e2e_client.py to e2e_db.py (implementation unchanged); the original module drops the definition, its __all__ entry, and the now-unused os import.

Confidence Score: 5/5

Safe to merge — changes are confined to the e2e test harness, add no new network calls, and the only behavioral shift (requiring an explicit opt-in before truncating spend logs) is strictly safer than the prior default-truncate behavior.

The destructive truncate is now correctly double-gated: the guard in run_spend_log_cleanup requires opt_in == "1" AND e2e_test_ran, and the old sys.path mutation is gone. The implementation in e2e_db.py is a clean extraction of logic that was already proven correct in spend_e2e_client.py. No production code is touched.

No files require special attention.

_{Reviews (4): Last reviewed commit: "test(e2e): guard destructive spend-log t..." | Re-trigger Greptile}

Greptile Summary

This PR guards the destructive LiteLLM_SpendLogs truncation behind an explicit E2E_RESET_SPEND_LOGS=1 opt-in, preventing routine e2e runs from wiping spend data on shared or staging databases. It also eliminates the sys.path hack by moving reset_spend_logs into a top-level e2e_db.py module and unit-testing the gate logic without a live DB.

• run_spend_log_cleanup in e2e_db.py requires both opt_in == \"1\" and e2e_test_ran == True; test_e2e_db.py covers all seven opt-in/test-ran combinations with a spy callable.
• The refactor removes the early return in pytest_sessionfinish that previously also guarded the bob_the_builder.remediate(session) call; remediate now runs unconditionally on every session finish, not only when an e2e-marked test ran.
• run_spend_log_cleanup returns True whether truncate() succeeded or raised, so callers cannot distinguish a successful truncation from a silently-failed one.

Confidence Score: 3/5

Safe to merge for the spend-log guard itself; the unintended un-gating of bob_the_builder.remediate needs a second look before landing.

The core opt-in guard is correct and well-tested. The main concern is in conftest.py: removing the early return means bob_the_builder.remediate(session) now runs at the end of every session, including pure harness-unit runs where no e2e-marked test executed. If remediate has external side effects, they will fire unexpectedly on unit-only runs.

tests/e2e/conftest.py — specifically whether bob_the_builder.remediate should remain behind the 'an e2e test ran' guard.

_{Reviews (1): Last reviewed commit: "test(e2e): guard destructive spend-log t..." | Re-trigger Greptile}

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Addressed the P1 in 0f1d45f. You're right that dropping the early return un-gated bob_the_builder.remediate. Restored its original gating: pytest_sessionfinish now reads e2e_test_ran once, passes it to run_spend_log_cleanup (which already no-ops when it's false), and returns before the remediate block when no e2e test ran, so remediate keeps running only on real e2e sessions exactly as before. Also strengthened test_cleanup_swallows_truncate_failure to assert the injected truncate was actually invoked (calls == 1) when it raises, not just that the return value is True.

@greptileai please review the current head 0f1d45f

The misc / Run tests red is a pre-existing failure on the base branch, not this PR. It's tests/test_litellm/test_gpt_realtime_mode.py::test_get_model_info_reports_realtime_mode failing with assert 'chat' == 'realtime'; that test fails identically when run on litellm_internal_staging at its current tip (get_model_info('gpt-realtime-mini')['mode'] returns chat, not realtime). This branch is a tests/e2e-only change (git diff origin/litellm_internal_staging..HEAD lists nothing outside tests/e2e/), so it cannot affect that model-info assertion; CI evaluates the PR merged into staging, which is how the base breakage surfaces here. It belongs in a separate fix on the base branch

Merging this PR will not alter performance

✅ 31 untouched benchmarks

_{Comparing litellm_lit4555_e2e_spend_truncate_guard (a2636fa) with litellm_internal_staging (e238e89)}

@greptileai please review the current head a2636fa — the branch was rebased onto current staging and the spend-log truncate guard was reapplied as a single commit; the prior review is pinned to a now-rewritten commit